Refactor and allow some support of extra-index-urls again #5028
Conversation
|
@matteius I don't feel like I have the expertise or knowledge to properly evaluate if this is the correct course. To my mind it does make more sense, but any security concerns should have priority I feel. |
|
I think there are too many edge cases of trying to support the --extra-index-url as it was before -- for example, when pypi isn't actually your primary index or pypi mirror this logic breaks down or has potential edge cases I haven't considered. If you look at my other linked PR that improves the documentation and potentially features around named and pinned indexes, that is where I think the real effort should be expended. |
|
@matteius Thank you for your information Let me explain our usecase in detail. We think many users have similar usecase. We have private index, and this private index cannnot be access without access token. We want to install private packages from our private index and also public packages from pypi. After reading your comment and PR https://github.com/pypa/pipenv/pull/5029/files#diff-67eeccaffd4730cc19ea43afd8caafcf84db895d57ac27bf10e0ad1e002ce17c, we have understood how we should use. Create Pipfile pipenv installAdd private index entry to Pipfile manually Pipfile [[source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"
[[source]]
url = "https://${MY_CREDENTIAL}@mydomain/simple"
verify_ssl = true
name = "myindexname"
[packages]
[dev-packages]
[requires]
python_version = "3.8"Install packages # To install public package, run the following command.
pipenv install requests
# To install private package, run the following command.
pipenv install --index=myindexname private-packageWe have tested this sequence using pipenv==2022.3.28 and that works fine! I would like to add the reason why we didn't use this flow and encounterd the issue #5021. We created Pipfile by running pipenv installAnd edit Pipfile as follows. Then, we could install our private packages using pipenv==v2020.11.15. pipenv install private-packageThis command succeeded, but recently we noticed that we got broken Pipfile.lock. Pipfile.lock ...
"default": {
"private-package": {
"hashes": [
"{hash}",
"{hash}"
],
"index": "pypi",
"version": "==0.x.y"
},
...After updating pipenv, we have encounterd the following issues and reported. I'm wondoring if some users ,like us, misunderstand how to use pipenv with their private index. For example, add this statement to https://pipenv.pypa.io/en/latest/advanced/#specifying-package-indexes ...
You can install requests from the "home" index by running the following command.
pipenv install --index=home requests
...And we noticed that the help message of I also think improving documentation leads many users the best solution, as you mentioned. |
|
@masato-yasuda Thank you for your useful feedback. I have added the essence of what you have captured in the specifying indexes section in this open PR: #5029 Could you indicate more what additional detail you are looking for in the section on credentials with environment variables? I will look into what you mean about the install help text. |
|
@matteius Thank you for your comment & updates. Let me share my team member's answer about your question. For the section on credentials, I thought that the similar statement will help users if exists. However, It might not be necessary. |
This PR tries to strike a balance between enforcing security in the new package restrictions, while still allowing extra indexes be specified loosely. I am not convinced that we should be doing this longer term -- in other words I think we should be aiming to pin our packages to specifically which index the package should need to be pulled from, a specific index if not the primary index, which is usually pypi.
As the pip documentation points out:
There is another caveat to not pinning your index explicitly, in the case of Torch. The caveat is that in this current incantation of the pipenv code, the pinned index correctly picks up all 8 hashes from the pinned index; however, if you rely on the extra index searching to find your dependency in the extra index (without explicitly pinning the requirement to the alternate index) it for some reason only pulls in the 1 hash that matches your system architecture out of the 8. I am not sure that is going to be sorted out readily as the hash aggregation logic is, well complicated and dependent on other factors in the resolution process as well.
I wanted to put this out there as a fix for the reported issues in the near term and gather feedback on what the real use case is for not pinning the index requirement in the first place? Note that the logic implicitly pins all packages with a non-specified index to be the default, which is usually pypi. So you can see how just having a second extra index here with this change makes all unpinned packages less predictable as to where they will be pulled from--it at least becomes not explicit.
The issue
Fixes #5022
Fixes #5021
The fix
@DanielPerezJensen @Bananaman and @masato-yasuda Could you review my concerns and potential change and provide feedback on the issues at hand. This technically fixes the issue being reported of the package not resolving when the index is not explicitly pinned, but I don't love the caveat that the hash collection is now incomplete when relying on this feature (I'm not entirely sure why yet). Plus I don't want to invest a lot of time into supporting it when there exists the security advisory from last year, it feels ultimately safer to be explicit in pinning indexes. Should we accept this patch? Will it really be improving quality of life and a legitimate use case, or will it be weakening the security of the tool? I am willing to consider it, since that was the prior behavior--but also willing to consider longer term deprecating it in favor of always pinning the non primary index packages to a specific index by name and potentially improving what CLI arguments are available to help manage this.
The checklist
news/directory to describe this fix with the extension.bugfix,.feature,.behavior,.doc..vendor. or.trivial(this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.